Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use key services and data keys where possible for faster EC operations. #38101

Closed
wants to merge 1 commit into from

Conversation

vcsjones
Copy link
Member

This is an attempt at improving asymmetric cryptographic operations on macOS, starting with ECDSA.

The makes use of Apple's newer Trust / Key Services APIs instead of using security transforms. In order to gain performance increases, two things needed to happen. One, the key needed to be a "data" key, not attached to any keychain, not even an ephemeral one. Second, use the SecKey* APIs.

Though the APIs still work off of SecKeyRefs, many of the legacy APIs cannot operate on keys that are not in a keychain, ephemeral or not. An example of this is SecItemExport for ECDSA data keys. To keep the existing code paths working and the changes more minimal, and allow for more gradual adoption of key services where possible, both key handle types are used. When data keys are present, those are preferred for signing and verification operations.

In this PR data keys exist in two scenarios. The first is Create or GenerateKey. The second is ImportParameters and any other import methods that defer to it. Other means of obtaining a key, like from a certificate in a keychain or importing a PFX, will not have data keys and will use security transforms.

The data keys are obtained by exporting the keys to ECParameters and importing them again through keychain services APIs. This means that although signing and verifying ECDSA operations are much faster, importing and key generation take a performance hit since they have more work to do.

Some native support for RSA exists here. Though none of the managed implementation uses it yet, as this is solely for ECDSA so far.

Contributes to #36107

Benchmarks:

All sign / verify benches use a generated key.

Method Job Toolchain Config Mean Error StdDev Ratio
SignHash Job-FCMYUV fast-ecdsa nistP256, SHA256 283.1 μs 3.70 μs 3.46 μs 0.05
SignHash Job-CJQWDK master nistP256, SHA256 5,303.6 μs 22.00 μs 19.50 μs 1.00
VerifyHash Job-FCMYUV fast-ecdsa nistP256, SHA256 235.1 μs 3.09 μs 2.89 μs 0.02
VerifyHash Job-CJQWDK master nistP256, SHA256 10,176.4 μs 42.18 μs 37.40 μs 1.00
SignHash Job-FCMYUV fast-ecdsa nistP384, SHA384 784.0 μs 9.87 μs 9.23 μs 0.08
SignHash Job-CJQWDK master nistP384, SHA384 9,659.1 μs 30.62 μs 28.65 μs 1.00
VerifyHash Job-FCMYUV fast-ecdsa nistP384, SHA384 643.1 μs 8.74 μs 8.18 μs 0.03
VerifyHash Job-CJQWDK master nistP384, SHA384 18,932.7 μs 39.28 μs 32.80 μs 1.00
SignHash Job-FCMYUV fast-ecdsa nistP521, SHA512 1,104.3 μs 13.61 μs 12.73 μs 0.06
SignHash Job-CJQWDK master nistP521, SHA512 17,658.8 μs 59.31 μs 55.48 μs 1.00
VerifyHash Job-FCMYUV fast-ecdsa nistP521, SHA512 841.2 μs 5.21 μs 4.35 μs 0.02
VerifyHash Job-CJQWDK master nistP521, SHA512 34,376.4 μs 113.74 μs 106.40 μs 1.00
Method Job Toolchain curve Mean Error StdDev Ratio
KeyGen Job-FCMYUV fast-ecdsa nistP256 65.20 ms 0.528 ms 0.494 ms 1.09
KeyGen Job-CJQWDK master nistP256 60.09 ms 0.368 ms 0.326 ms 1.00
KeyGen Job-FCMYUV fast-ecdsa nistP384 96.29 ms 0.595 ms 0.557 ms 1.04
KeyGen Job-CJQWDK master nistP384 92.89 ms 0.737 ms 0.689 ms 1.00
KeyGen Job-FCMYUV fast-ecdsa nistP521 151.86 ms 0.676 ms 0.632 ms 1.04
KeyGen Job-CJQWDK master nistP521 146.65 ms 0.874 ms 0.817 ms 1.00
Method Job Toolchain curve Mean Error StdDev Ratio RatioSD
ImportPrivateParameters Job-FCMYUV fast-ecdsa nistP256 24,156.73 μs 208.931 μs 195.434 μs 1.32 0.01
ImportPrivateParameters Job-CJQWDK master nistP256 18,260.68 μs 126.533 μs 118.359 μs 1.00 0.00
ImportPublicParameters Job-FCMYUV fast-ecdsa nistP256 500.94 μs 6.068 μs 5.676 μs 2.36 0.03
ImportPublicParameters Job-CJQWDK master nistP256 212.29 μs 0.567 μs 0.503 μs 1.00 0.00
ExportPrivateParameters Job-FCMYUV fast-ecdsa nistP256 4,785.72 μs 20.711 μs 19.373 μs 0.99 0.00
ExportPrivateParameters Job-CJQWDK master nistP256 4,813.65 μs 19.685 μs 17.450 μs 1.00 0.00
ExportPublicParameters Job-FCMYUV fast-ecdsa nistP256 63.79 μs 0.260 μs 0.243 μs 1.02 0.01
ExportPublicParameters Job-CJQWDK master nistP256 62.81 μs 0.387 μs 0.362 μs 1.00 0.00
ImportPrivateParameters Job-FCMYUV fast-ecdsa nistP384 40,911.78 μs 189.020 μs 157.841 μs 1.12 0.01
ImportPrivateParameters Job-CJQWDK master nistP384 36,636.35 μs 334.361 μs 296.402 μs 1.00 0.00
ImportPublicParameters Job-FCMYUV fast-ecdsa nistP384 494.84 μs 1.100 μs 0.859 μs 2.35 0.01
ImportPublicParameters Job-CJQWDK master nistP384 210.55 μs 0.663 μs 0.620 μs 1.00 0.00
ExportPrivateParameters Job-FCMYUV fast-ecdsa nistP384 4,793.74 μs 17.751 μs 13.859 μs 0.99 0.01
ExportPrivateParameters Job-CJQWDK master nistP384 4,820.73 μs 26.000 μs 23.049 μs 1.00 0.00
ExportPublicParameters Job-FCMYUV fast-ecdsa nistP384 62.36 μs 0.344 μs 0.322 μs 0.98 0.01
ExportPublicParameters Job-CJQWDK master nistP384 63.75 μs 0.275 μs 0.215 μs 1.00 0.00
ImportPrivateParameters Job-FCMYUV fast-ecdsa nistP521 73,257.64 μs 930.959 μs 870.820 μs 1.07 0.02
ImportPrivateParameters Job-CJQWDK master nistP521 68,366.20 μs 874.672 μs 818.169 μs 1.00 0.00
ImportPublicParameters Job-FCMYUV fast-ecdsa nistP521 504.41 μs 1.454 μs 1.360 μs 2.26 0.10
ImportPublicParameters Job-CJQWDK master nistP521 220.97 μs 4.129 μs 7.551 μs 1.00 0.00
ExportPrivateParameters Job-FCMYUV fast-ecdsa nistP521 4,808.95 μs 18.758 μs 16.628 μs 0.96 0.01
ExportPrivateParameters Job-CJQWDK master nistP521 5,005.39 μs 37.495 μs 35.073 μs 1.00 0.00
ExportPublicParameters Job-FCMYUV fast-ecdsa nistP521 63.73 μs 0.298 μs 0.279 μs 0.97 0.02
ExportPublicParameters Job-CJQWDK master nistP521 65.62 μs 1.293 μs 1.681 μs 1.00 0.00

Code is here: https://github.com/vcsjones/DotNetCryptoBenches

@ghost
Copy link

ghost commented Jun 18, 2020

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq
Notify danmosemsft if you want to be subscribed.

@vcsjones
Copy link
Member Author

I guess what I am looking to get input on:

  1. Are the performance gains worth the introduced complexity of essentially supporting two different sign / verify paths on macOS?
  2. Are the performance gains relevant to where data keys exist? e.g. generated and imported? Or do we expect most keys to come from PFX / keychains?
    1. This may be possible for PFX and in some cases keychain keys. The limiting factor is that the private key needs to exportable. If we can't do that, we can't key it in to key services.
  3. Are the performance losses on generate and import acceptable?

Copy link
Member

@bartonjs bartonjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things seem.... OK... though I'm confused now about what the difference is between new and old, and am curious to see if things can collapse (when I looked previously I thought they were completely parallel key types, not both SecKeyRef).

If the big difference is that now (vs when I wrote all of this) keys work without being stored in the keychain, but PFX export fails unless it's in a keychain, then I'd hope for an approach like:

  • KeyGen makes only the purely ephemeral keys ("data keys"?)
  • I assume creating a SecIdentityRef still requires a keychain, so CopyWithPrivateKey would need to find out that the private key doesn't have a keychain and deal with that then.
  • The Get*PublicKey methods on a cert should just export the keychain-based key and return a new ephemeral key, it looks like that's always guaranteed to make the first public key usage faster, by a couple orders of magnitude.
  • The Get*PrivateKey methods can opportunistically export the keychain-based key and make it into an ephemeral keypair. If export fails, use an ephemeral public key along with a keychain private key.

Assuming that all keys can go through the simpler-looking API, we'd just route them all there. If the new API isn't available on iOS, but the old one is, we can hopefully use the one signature and make it use the new API when available and the old API when required. Then all the complexities are around key->cert and cert->key, but once we have a key everything makes sense.

{
Success => dataKey,
kErrorSeeError => throw CreateExceptionForCFError(errorHandle),
_ => throw new CryptographicException { HResult = result }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unexpected error codes should Debug.Fail, then probably just throw a default CryptographicException (consistency with the existing paths).

SecKeyPair keys = GetKeys();
byte[] formattedSignature = AsymmetricAlgorithmHelpers.ConvertIeee1363ToDer(signature);

if (keys.PublicDataKey != null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would there ever be a situation where we can't get a public data key?


private SecKeyPair(SafeSecKeyRefHandle publicKey, SafeSecKeyRefHandle? privateKey)
private SecKeyPair(SafeSecKeyRefHandle publicKey, SafeSecKeyRefHandle? privateKey, SafeSecKeyRefHandle? publicDataKey, SafeSecKeyRefHandle? privateDataKey)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the SecKeyRef version of the public key needed for (abstractly)? It seems like all public keys are exportable, all exportable keys can be made into data keys, and data keys are faster.

Can this pair type be a triplet instead of a quadret? PublicDataKey / PrivateDataKey / PrivateKey


private SecKeyPair(SafeSecKeyRefHandle publicKey, SafeSecKeyRefHandle? privateKey)
private SecKeyPair(SafeSecKeyRefHandle publicKey, SafeSecKeyRefHandle? privateKey, SafeSecKeyRefHandle? publicDataKey, SafeSecKeyRefHandle? privateDataKey)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, now I'm confused. If the existing keys were SecKeyRefs, and the new keys are SecKeyRefs, what's the difference, and why don't they interoperate?

I'm not finding the data in the docs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's... hard to articulate and compounded by I think there is at least one bug in MacOS that I am trying to get chased down. I'll experiment a bit and come back with a more thorough explanation of things.

Now that I have Big Sur working and runtime building on it (surprise, nothing in S.S.C broke so far) I wonder if I will still hit quirks.

@vcsjones
Copy link
Member Author

Going to close this for now, will make a new PR when it makes sense.

@vcsjones vcsjones closed this Jul 10, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants